Fix: Auto-detect unprivileged user and use XDG_RUNTIME_DIR for default root#465
Conversation
✅ Deploy Preview for urunc canceled.
|
|
Hello @sidneychang , thank you for this PR. The idea looks good, but maybe we could make use of urunc;s configuration instead of the |
cmainas
left a comment
There was a problem hiding this comment.
Thank you @sidneychang for this addition. I have added a few comments to slightly improve the code.
pkg/rootless/rootless.go
Outdated
| // for the default root directory (e.g. /run/user/UID/runc instead of /run/urunc). | ||
| // It returns true for non-root processes and for root inside a user namespace | ||
| // when not running as the "root" user (e.g. rootless Podman). | ||
| func ShouldHonorXDGRuntimeDir() bool { |
There was a problem hiding this comment.
Let's add this function in https://github.com/urunc-dev/urunc/blob/main/cmd/urunc/utils.go to avoid creating a new package.
cmd/urunc/main.go
Outdated
|
|
||
| func main() { | ||
| root := "/run/urunc" | ||
| xdgDirUsed := false |
There was a problem hiding this comment.
We can move this code inside the if !cmd.IsSet("root") statement and then perform the check if we should use the XDG_RUNTME_DIR
cmd/urunc/main.go
Outdated
| // auto-pruned. | ||
| if err := os.MkdirAll(root, 0o700); err != nil { | ||
| fmt.Fprintln(os.Stderr, "the path in $XDG_RUNTIME_DIR must be writable by the user") | ||
| fatal(err) |
There was a problem hiding this comment.
Avoid using fatal. Return the error instead.
cmd/urunc/main.go
Outdated
| } | ||
| if err := os.Chmod(root, os.FileMode(0o700)|os.ModeSticky); err != nil { | ||
| fmt.Fprintln(os.Stderr, "you should check permission of the path in $XDG_RUNTIME_DIR") | ||
| fatal(err) |
There was a problem hiding this comment.
Avoid using fatal. Return the error instead.
cmd/urunc/main.go
Outdated
| }, | ||
| Before: func(_ context.Context, cmd *cli.Command) (context.Context, error) { | ||
| if !cmd.IsSet("root") && xdgDirUsed { | ||
| // According to the XDG specification, we need to set anything in |
There was a problem hiding this comment.
We can move the XDG_RUNTIME_DIR preparation in a single small function.
9b6dac5 to
248a6c6
Compare
|
Hi @cmainas, Thanks for the review — all comments have been addressed. |
248a6c6 to
aa45041
Compare
|
Thank you @sidneychang for the changes. Could you rebase over the main branch, so we can merge it? |
f1bab24 to
a8ceeb2
Compare
|
Hi @cmainas, |
|
Thank you @sidneychang , would it be possible to rebase once more, just to be on the safe side? |
a8ceeb2 to
441b801
Compare
Mirror runc behavior: when running as non-root (or root inside a user namespace), honor $XDG_RUNTIME_DIR for the default runtime root so callers do not need to pass --root. Signed-off-by: sidneychang <2190206983@qq.com>
|
To fix the linter issue with spelling, you will need to add the word |
441b801 to
a1b89e5
Compare
|
Hi @cmainas, The other CI checks look good, but There’s also a failure in |
cmainas
left a comment
There was a problem hiding this comment.
Thank you @sidneychang for this addition!
PR: #465 Mirror runc behavior: when running as non-root (or root inside a user namespace), honor $XDG_RUNTIME_DIR for the default runtime root so callers do not need to pass --root. Signed-off-by: sidneychang <2190206983@qq.com> Reviewed-by: Charalampos Mainas <cmainas@nubificus.co.uk> Approved-by: Charalampos Mainas <cmainas@nubificus.co.uk>
PR: #465 Mirror runc behavior: when running as non-root (or root inside a user namespace), honor $XDG_RUNTIME_DIR for the default runtime root so callers do not need to pass --root. Signed-off-by: sidneychang <2190206983@qq.com> Reviewed-by: Charalampos Mainas <cmainas@nubificus.co.uk> Approved-by: Charalampos Mainas <cmainas@nubificus.co.uk>
Description
This change mirrors runc’s behavior when running as a non-root user (or as root inside a user namespace). In these cases, the runtime now automatically falls back to using $XDG_RUNTIME_DIR as the default runtime root, instead of requiring callers to explicitly pass --root.
By honoring $XDG_RUNTIME_DIR by default in these environments, urunc now aligns with runc’s behavior and integrates more smoothly with Podman.
Related issues
How was this tested?
LLM usage
N/A
Checklist
make lint).make test_ctr,make test_nerdctl,make test_docker,make test_crictl).